Fix integer overflow in compute_numel() by lucylq · Pull Request #18598 · pytorch/executorch

lucylq · 2026-03-30T23:58:32Z

compute_numel() multiplies tensor dimensions without overflow protection. The result is used for size calculations in make_tensor_ptr() and clone_tensor_ptr(), so an overflow could lead to undersized allocations and subsequent buffer overflows.

This PR was authored with the assistance of Claude.

--
TensorImpl ctor can't return an error. To avoid aborting when calculating numel (because of overflow), we have to introduce a new ctor that takes numel as a precomputed value.

Update the tensor ctor to take in numel. Deprecate the old one following cycle.
Make compute_numel() return a Result.
Update all callsites to use the new tensor ctor (~15 across extension and other places)
tensor_ptr and tensor_ptr_maker call compute_numel() directly in their ctor. They also need to be updated with either a new ctor, or use ET_CHECK_MSG.

The blast radius is a bit large. I think we can do this for now and tackle the ET_CHECK_MSG removal across the codebase separately.

pytorch-bot · 2026-03-30T23:58:36Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/18598

📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

Rolling out OSDC (ARC) runners on pull & trunk workflows in PyTorch main

❌ 1 New Failure, 3 Unrelated Failures

As of commit 40bfc44 with merge base 0919746 ():

NEW FAILURE - The following job has failed:

pull / android / run-emulator (gh)
The process '/usr/local/lib/android/sdk/platform-tools/adb' failed with exit code 224

FLAKY - The following job failed but was likely due to flakiness present on trunk:

pull / test-qnn-testsuite-linux / test-backend-linux (qnn, models) / linux-job (gh) (detected as infra flaky with no log or failing log classifier)

BROKEN TRUNK - The following jobs failed but was present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

pull / unittest / windows / windows-job (gh) (trunk failure)
##[error]The operation was canceled.
pull / unittest-editable / windows / windows-job (gh) (trunk failure)
##[error]The operation was canceled.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

github-actions · 2026-03-30T23:59:21Z

This PR needs a `release notes:` label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

lucylq · 2026-04-01T00:02:51Z

+        InvalidProgram,
+        "Overflow computing numel at dim %zu",
+        static_cast<size_t>(i));
+    numel *= sizes[i];


It's a double calculation, but ensures we don't abort when calculating numel() from untrusted tensor at parsing time.

Copilot

Pull request overview

This PR aims to prevent integer overflow while computing tensor numel, which is used for downstream size calculations (and can otherwise lead to undersized allocations and potential buffer overflows).

Changes:

Add ssize_t overflow checks when multiplying tensor dimensions during tensor deserialization.
Add overflow protection inside compute_numel() used by TensorImpl.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
runtime/executor/tensor_parser_portable.cpp	Adds validation for negative sizes and overflow while computing `numel` during FlatBuffer tensor parsing.
runtime/core/portable_type/tensor_impl.cpp	Adds an overflow check in `compute_numel()` to prevent `ssize_t` wrap/UB when multiplying dimensions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-01T00:06:05Z

+        "Overflow computing numel at dim %zu",
+        static_cast<size_t>(i));
+    numel *= sizes[i];
  }


This guards overflow for numel, but tensor_impl->nbytes() later multiplies numel by elementSize(scalar_type); that multiplication can still overflow size_t and wrap, leading to undersized allocation/reads. Consider adding a byte-size overflow check here (or in TensorImpl::nbytes()) using SIZE_MAX / elementSize(scalar_type) to ensure the computed nbytes is safe.

Suggested change

}

}

// Also ensure that the total number of bytes fits in size_t when

// multiplied by the element size for this scalar type.

const size_t element_size = elementSize(scalar_type);

ET_CHECK_OR_RETURN_ERROR(

numel == 0 ||

static_cast<size_t>(numel) <= SIZE_MAX / element_size,

InvalidProgram,

"Overflow computing tensor nbytes");

lucylq · 2026-04-01T18:36:30Z

+        "Overflow computing numel: %zd * %zd would overflow ssize_t at dimension %zd",
+        numel,
+        static_cast<ssize_t>(sizes[i]),
+        i);


We should remove ET_CHECK_MSG, this has a large blast radius here though (see PR summary).

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

JacobSzwejbka · 2026-04-22T18:05:09Z

        static_cast<ssize_t>(sizes[i]),
        i);
+    ET_CHECK_MSG(
+        sizes[i] == 0 || numel <= (ssize_t)(SIZE_MAX / 2) / sizes[i],


Can you just use the same overflow c10 check we use other places?

compute_numel() multiplies tensor dimensions without overflow protection. The result is used for size calculations in make_tensor_ptr() and clone_tensor_ptr(), so an overflow could lead to undersized allocations and subsequent buffer overflows. Add an ET_CHECK_MSG before each multiplication to verify that numel * sizes[i] will not exceed SSIZE_MAX. This PR was authored with the assistance of Claude.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-22T21:10:14Z

 #include <algorithm>
+#include <climits>
 #include <cstdint>

 #include <c10/util/irange.h>
+#include <c10/util/safe_numerics.h>


<climits> appears unused in this translation unit. If you need bounds for ssize_t, prefer <limits> + std::numeric_limits<ssize_t>; otherwise, drop the include to reduce churn.

meta-codesync · 2026-04-22T21:16:35Z

@lucylq has imported this pull request. If you are a Meta employee, you can view this in D102049291.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-22T22:06:22Z

+    ET_CHECK_MSG(
+        !c10::mul_overflows(numel, static_cast<ssize_t>(sizes[i]), &next_numel),
+        "Overflow computing numel: %zd * %zd would overflow ssize_t at dimension %zd",
+        numel,
+        static_cast<ssize_t>(sizes[i]),
+        i);


The overflow ET_CHECK_MSG uses %zd for the dimension index, but i comes from c10::irange(dim) (typically an int64_t). With printf-style formatting this mismatch is undefined behavior on some platforms. Cast i to ssize_t (or switch to the correct format macro/specifier) when passing it to the format string.

Copilot · 2026-04-22T22:06:23Z

 #include <executorch/runtime/core/portable_type/tensor_impl.h>

 #include <algorithm>
+#include <climits>


#include <climits> was added but doesn't appear to be used in this file. Consider removing it to avoid unnecessary dependencies.

Suggested change

#include <climits>

Copilot · 2026-04-22T22:06:23Z

+    ssize_t next_numel;
+    ET_CHECK_OR_RETURN_ERROR(
+        !c10::mul_overflows(numel, static_cast<ssize_t>(sizes[i]), &next_numel),
+        InvalidProgram,
+        "Overflow computing numel at dim %zu",
+        static_cast<size_t>(i));
+    numel = next_numel;


Similar to TensorImpl::compute_numel(): this calls c10::mul_overflows with signed ssize_t. On MSVC the fallback implementation multiplies signed values directly, which can be UB when overflow occurs. Since tensor sizes are validated as non-negative here, use an unsigned accumulator for the overflow check (and separately ensure the final product fits in ssize_t) to make the check well-defined across compilers.

lucylq · 2026-04-22T22:06:35Z

causes some internal failures. Let's rely on D98148157

meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 30, 2026

lucylq changed the title ~~Fix integer overflow in compute_numel() (TOB-EXECUTORCH-19)~~ Fix integer overflow in compute_numel() Mar 30, 2026

lucylq force-pushed the security19 branch 7 times, most recently from 0d38e7f to 31330a6 Compare March 31, 2026 23:56

lucylq marked this pull request as ready for review April 1, 2026 00:01

lucylq requested a review from JacobSzwejbka as a code owner April 1, 2026 00:01

Copilot AI review requested due to automatic review settings April 1, 2026 00:01

Copilot started reviewing on behalf of lucylq April 1, 2026 00:02 View session

lucylq commented Apr 1, 2026

View reviewed changes

Copilot AI reviewed Apr 1, 2026

View reviewed changes

lucylq force-pushed the security19 branch from 31330a6 to 2ab7fa9 Compare April 1, 2026 01:40

Copilot AI review requested due to automatic review settings April 1, 2026 18:35

lucylq force-pushed the security19 branch from 2ab7fa9 to f15bf18 Compare April 1, 2026 18:35

lucylq commented Apr 1, 2026

View reviewed changes

Copilot started reviewing on behalf of lucylq April 1, 2026 18:36 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

lucylq added the security-fix label Apr 2, 2026

JacobSzwejbka approved these changes Apr 22, 2026

View reviewed changes

lucylq force-pushed the security19 branch from f15bf18 to 638b189 Compare April 22, 2026 21:03

et check

043baf7

Copilot AI review requested due to automatic review settings April 22, 2026 21:06

lucylq force-pushed the security19 branch from 638b189 to 043baf7 Compare April 22, 2026 21:06

Copilot started reviewing on behalf of lucylq April 22, 2026 21:07 View session

Copilot AI reviewed Apr 22, 2026

View reviewed changes

Apply suggestion from @Copilot

40bfc44

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings April 22, 2026 22:01

Copilot started reviewing on behalf of lucylq April 22, 2026 22:02 View session

Copilot AI reviewed Apr 22, 2026

View reviewed changes

lucylq closed this Apr 24, 2026

-  }
+  }
+  // Also ensure that the total number of bytes fits in size_t when
+  // multiplied by the element size for this scalar type.
+  const size_t element_size = elementSize(scalar_type);
+  ET_CHECK_OR_RETURN_ERROR(
+      numel == 0 ||
+          static_cast<size_t>(numel) <= SIZE_MAX / element_size,
+      InvalidProgram,
+      "Overflow computing tensor nbytes");

Conversation

lucylq commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot Bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/18598

❗ 1 Active SEVs

❌ 1 New Failure, 3 Unrelated Failures

Uh oh!

github-actions Bot commented Mar 30, 2026

This PR needs a release notes: label

Uh oh!

lucylq Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lucylq Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

JacobSzwejbka Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

meta-codesync Bot commented Apr 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

lucylq commented Apr 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lucylq commented Mar 30, 2026 •

edited

Loading

pytorch-bot Bot commented Mar 30, 2026 •

edited

Loading

This PR needs a `release notes:` label